Skip to content

Improve static asserts#642

Closed
AlexInLog wants to merge 4 commits into
v2from
static_asserts
Closed

Improve static asserts#642
AlexInLog wants to merge 4 commits into
v2from
static_asserts

Conversation

@AlexInLog

@AlexInLog AlexInLog commented Sep 18, 2024

Copy link
Copy Markdown
Owner

Summary by CodeRabbit

  • New Features

    • Enhanced type safety and clarity in various operators through updated constraints and assertions.
    • Introduced new concepts for callable types, improving the robustness of type checks.
  • Bug Fixes

    • Improved compile-time error messaging for type constraints in several operators, preventing potential runtime errors.
  • Documentation

    • Clarified parameter names and descriptions in the reduce and scan functions for better understanding.
  • Refactor

    • Streamlined constraints in multiple operators, enhancing maintainability and readability of the code.
  • Chores

    • Removed outdated concepts from the constraints namespace to simplify the codebase.

@coderabbitai

coderabbitai Bot commented Sep 18, 2024

Copy link
Copy Markdown
Contributor
Walkthrough

Walkthrough

The pull request introduces significant modifications across various files in the rpp library, focusing on enhancing type safety and clarity through the introduction of new constraints and assertions. Key changes include the addition of static_assert statements to enforce callable requirements, the refinement of existing constraints, and the introduction of new concepts to streamline type checking. These updates aim to improve the maintainability and robustness of the codebase, particularly in the context of reactive programming.

Changes

Files Change Summary
src/rpp/rpp/disposables/fwd.hpp Added #include <rpp/utils/function_traits.hpp>.
src/rpp/rpp/operators/combine_latest.hpp Modified constraints for combine_latest to use constraint::template_callable_or_invocable.
src/rpp/rpp/operators/distinct_until_changed.hpp Updated static assertions for EqualityFn to ensure it returns a boolean type.
src/rpp/rpp/operators/filter.hpp Added static_assert for predicate type constraints in filter.
src/rpp/rpp/operators/flat_map.hpp Introduced static_assert to validate callable type Fn.
src/rpp/rpp/operators/fwd.hpp Updated constraints for multiple operators, including combine_latest, distinct_until_changed, etc.
src/rpp/rpp/operators/group_by.hpp Added static assertions for KeySelector, ValueSelector, and KeyComparator.
src/rpp/rpp/operators/map.hpp Changed from requires clause to static_assert for callable Fn.
src/rpp/rpp/operators/merge.hpp Replaced requires clause with static_assert for type consistency in merge_with.
src/rpp/rpp/operators/on_error_resume_next.hpp Added static_assert for Selector type constraints.
src/rpp/rpp/operators/reduce.hpp Renamed parameter from initial_value to seed and added a static assertion for accumulator.
src/rpp/rpp/operators/retry_when.hpp Replaced requires clause with static_assert for TNotifier.
src/rpp/rpp/operators/scan.hpp Renamed parameters for clarity and updated static assertions.
src/rpp/rpp/operators/subscribe.hpp Modified on_next_like concept for improved type checking.
src/rpp/rpp/operators/take_while.hpp Added static_assert for predicate function's return type.
src/rpp/rpp/operators/tap.hpp Updated constraints for OnNext and OnError parameters.
src/rpp/rpp/operators/with_latest_from.hpp Simplified constraints for TSelector.
src/rpp/rpp/operators/zip.hpp Modified constraints for TSelector in zip.
src/rpp/rpp/sources/fwd.hpp Updated template constraints for OnSubscribe.
src/rpp/rpp/utils/constraints.hpp Removed invocable_r_v and is_nothrow_invocable concepts.
src/rpp/rpp/utils/function_traits.hpp Introduced new concepts for callable types and updated existing ones for clarity.

Possibly related PRs

  • asio strand scheduler #627: Introduces a new strand class utilizing ASIO for task scheduling, potentially related to the enhancements in type safety and callable constraints in this PR.

🐇 In the code we hop and play,
With types and traits, we pave the way.
Static asserts now keep us safe,
In reactive streams, we find our grace.
So let’s celebrate this code so bright,
A leap for safety, a joyful sight! 🌟


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    -- I pushed a fix in commit <commit_id>, please review it.
    -- Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    -- @coderabbitai generate unit testing code for this file.
    -- @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    -- @coderabbitai generate interesting stats about this repository and render them as a table.
    -- @coderabbitai read src/utils.ts and generate unit testing code.
    -- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    -- @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Outside diff range and nitpick comments (1)
src/rpp/rpp/operators/combine_latest.hpp (1)

102-102: Approve the usage of the new utility to simplify the constraint.

The change enhances the clarity and maintainability of the code by reducing complexity in the type constraints while ensuring that the functionality remains intact. The new utility constraint::template_callable_or_invocable effectively combines the previous checks into a single, more streamlined requirement.

Consider adding a comment to explain the purpose of the new utility for better readability. For example:

requires (!rpp::constraint::observable<TSelector> && (constraint::template_callable_or_invocable<TSelector, rpp::utils::convertible_to_any, utils::extract_observable_type_t<TObservable>, utils::extract_observable_type_t<TObservables>...>))
// ^-- The new utility checks if TSelector is a callable or invocable with the specified types
Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 0f86e39 and 03e5b14.

Files selected for processing (21)
  • src/rpp/rpp/disposables/fwd.hpp (1 hunks)
  • src/rpp/rpp/operators/combine_latest.hpp (1 hunks)
  • src/rpp/rpp/operators/distinct_until_changed.hpp (2 hunks)
  • src/rpp/rpp/operators/filter.hpp (1 hunks)
  • src/rpp/rpp/operators/flat_map.hpp (1 hunks)
  • src/rpp/rpp/operators/fwd.hpp (8 hunks)
  • src/rpp/rpp/operators/group_by.hpp (1 hunks)
  • src/rpp/rpp/operators/map.hpp (1 hunks)
  • src/rpp/rpp/operators/merge.hpp (1 hunks)
  • src/rpp/rpp/operators/on_error_resume_next.hpp (1 hunks)
  • src/rpp/rpp/operators/reduce.hpp (2 hunks)
  • src/rpp/rpp/operators/retry_when.hpp (1 hunks)
  • src/rpp/rpp/operators/scan.hpp (2 hunks)
  • src/rpp/rpp/operators/subscribe.hpp (1 hunks)
  • src/rpp/rpp/operators/take_while.hpp (1 hunks)
  • src/rpp/rpp/operators/tap.hpp (3 hunks)
  • src/rpp/rpp/operators/with_latest_from.hpp (1 hunks)
  • src/rpp/rpp/operators/zip.hpp (1 hunks)
  • src/rpp/rpp/sources/fwd.hpp (1 hunks)
  • src/rpp/rpp/utils/constraints.hpp (0 hunks)
  • src/rpp/rpp/utils/function_traits.hpp (1 hunks)
Files not reviewed due to no reviewable changes (1)
  • src/rpp/rpp/utils/constraints.hpp
Additional comments not posted (41)
src/rpp/rpp/disposables/fwd.hpp (1)

14-14: LGTM!

The addition of the include directive for <rpp/utils/function_traits.hpp> is a valid change. It is likely to be used in the implementation files to enhance the capabilities of the code by leveraging type traits associated with functions.

src/rpp/rpp/operators/flat_map.hpp (1)

69-69: Excellent addition of static_assert for improved type safety and clarity!

The introduction of the static_assert statement enhances the type safety of the flat_map function by explicitly checking the validity of the callable type Fn. This compile-time check ensures that Fn, if not a template callable, must return an observable type when invoked with a parameter of type rpp::utils::convertible_to_any.

This change provides several benefits:

  1. It prevents potential misuse of the flat_map function with incompatible types, catching errors at compile-time rather than runtime.
  2. It improves clarity and error messaging during compilation compared to the previous requires clause, making it easier for developers to understand and fix any issues.
  3. It enhances the maintainability and robustness of the codebase by enforcing strict type requirements.

Great work on this improvement!

src/rpp/rpp/operators/filter.hpp (1)

96-96: Excellent addition of static_assert for improved type safety!

The introduction of the static_assert is a great enhancement to the filter function. It enforces a clear and explicit constraint on the predicate parameter, ensuring that it is either a template callable or invocable with a return type of bool when applied to rpp::utils::convertible_to_any.

This compile-time check improves type safety by catching any incompatible predicate types early in the development process. It provides clearer error messages during compilation, making it easier to identify and fix issues related to incorrect predicate types.

The use of rpp::utils::convertible_to_any allows flexibility in the types that can be passed to the predicate, as long as they are convertible to the expected parameter type.

Overall, this change enhances the robustness and maintainability of the code by preventing potential runtime errors and improving the developer experience.

src/rpp/rpp/operators/map.hpp (1)

99-99: Excellent addition of static_assert to enforce callable constraints!

The introduction of the static_assert statement with the constraint::template_callable_or_invocable_ret_non_void concept effectively enforces the requirement for the callable Fn to be either a template callable or an invocable function that returns a non-void type when called with rpp::utils::convertible_to_any.

This change enhances type safety by catching any violations of the constraint at compile-time, preventing potential runtime errors related to void return types. The clear error message in the static_assert statement provides helpful feedback to developers if the constraint is not met, improving the maintainability and robustness of the code.

Great work on improving the type safety and clarity of the map function!

src/rpp/rpp/operators/take_while.hpp (1)

96-96: Excellent addition of static_assert to enforce predicate constraints!

The introduction of the static_assert statement enhances type safety by ensuring that the predicate function Fn adheres to the expected signature. This compile-time check prevents potential runtime errors related to incorrect function signatures and improves clarity regarding the expected behavior of the predicate.

The constraint enforces that Fn is either callable or invocable with a return type of bool when applied to rpp::utils::convertible_to_any, which is a meaningful and appropriate requirement for the take_while operator.

This change is a positive step towards improving the maintainability and robustness of the codebase.

src/rpp/rpp/utils/function_traits.hpp (10)

44-45: LGTM!

The namespace closure is correct.


46-51: LGTM!

The is_not_template_callable concept is correctly defined using the utils::is_not_template_callable_t type trait.


52-53: LGTM!

The invocable concept is correctly defined using the std::invocable concept.


55-56: LGTM!

The invocable_ret concept is correctly defined using the invocable concept and checking the return type using std::same_as and std::invoke_result_t.


58-59: LGTM!

The template_callable_or_invocable concept is correctly defined using the is_not_template_callable and invocable concepts.


61-62: LGTM!

The template_callable_or_invocable_ret_non_void concept is correctly defined using the is_not_template_callable, invocable, and invocable_ret concepts to check if a function is not a template callable, invocable, and does not return void.


64-65: LGTM!

The template_callable_or_invocable_ret concept is correctly defined using the is_not_template_callable and invocable_ret concepts to check if a function is not a template callable, invocable, and returns the specified Ret type.


67-68: LGTM!

The is_nothrow_invocable concept is correctly defined using the std::is_nothrow_invocable_v type trait to check if a function is invocable without throwing an exception.


69-72: LGTM!

The namespace closures and openings are correct.


75-75: LGTM!

The function_traits struct template is correctly modified to use the constraint::is_not_template_callable concept as a constraint for the template parameter T.

src/rpp/rpp/sources/fwd.hpp (1)

39-39: LGTM!

The change to use constraint::is_not_template_callable for the OnSubscribe template parameter is a good refactoring that improves code organization and clarity by moving the constraint to a more appropriate namespace.

The default type for Type remains unchanged, maintaining the existing behavior of extracting the observer type from the OnSubscribe callable.

Overall, the changes enhance the code without introducing any apparent issues or altering the core functionality of the create function template.

src/rpp/rpp/operators/distinct_until_changed.hpp (2)

58-58: Improved type safety and clarity in the static assertion.

The change from invocable_r_v to invocable_ret enhances type safety by explicitly checking that the return type of EqualityFn is a boolean when invoked with two parameters of type T. This stricter check ensures that the function signature adheres to the expected return type.

Additionally, the updated assertion message provides a clearer description of the expected function signature, improving the clarity of any compile-time errors related to this assertion.


102-102: New static assertion enhances type-checking and error messaging.

The addition of the new static assertion using template_callable_or_invocable_ret provides an additional layer of type-checking for EqualityFn. It ensures that EqualityFn is either callable or invocable with the specified types (rpp::utils::convertible_to_any) and returns a boolean.

This assertion reinforces the requirement that the function must return a boolean when invoked with the specified parameters, improving the robustness of the distinct_until_changed operator's implementation.

Furthermore, the clear assertion message stating the expected function signature will help in identifying and resolving any compile-time errors related to this requirement, enhancing the developer experience.

src/rpp/rpp/operators/on_error_resume_next.hpp (1)

141-141: Excellent addition of static_assert for improved type safety and clarity!

The introduction of the static_assert statement enhances the type safety and robustness of the on_error_resume_next function by explicitly checking that the provided Selector is invocable with a std::exception_ptr and returns an observable type.

This change provides clearer compile-time error messages compared to the previous implementation, which only relied on a requires clause. It helps prevent misuse of the function and improves the maintainability of the codebase.

src/rpp/rpp/operators/zip.hpp (1)

102-102: Improved constraint for TSelector template parameter.

The change to the constraint on the TSelector template parameter enhances code clarity and maintainability by consolidating the logic into a single, more descriptive constraint function, constraint::template_callable_or_invocable.

The new constraint still ensures that TSelector is not an observable and is either a callable template or invocable with the specified types, maintaining the same functionality as the original constraint.

src/rpp/rpp/operators/tap.hpp (3)

69-69: LGTM!

The change from invocable_r_v to invocable_ret enhances the type safety by ensuring that OnNext is not only invocable with T but also returns void. This aligns with the PR objective of improving static asserts and type safety.


93-93: Looks good!

Moving the is_not_template_callable constraint from the utils namespace to the constraint namespace improves the organization and clarity of the codebase. The behavior of the code remains unchanged.


160-160: Looks good!

As mentioned in the previous comment for line 93, moving the is_not_template_callable constraint from the utils namespace to the constraint namespace improves the organization and clarity of the codebase. The behavior of the code remains unchanged.

src/rpp/rpp/operators/reduce.hpp (2)

Line range hint 134-148: Documentation update looks good!

The parameter name change from initial_value to seed in the documentation improves clarity and consistency with the variable name used in the code.


148-148: Great addition of the static_assert!

The new static_assert statement enforces a compile-time check on the accumulator function, ensuring that it is callable with the seed type and returns the same type. This assertion enhances type safety and helps catch potential type mismatches at compile-time, improving the robustness of the code.

src/rpp/rpp/operators/retry_when.hpp (1)

195-195: Improved error message clarity with static_assert.

Replacing the requires clause with a static_assert is a good change. It provides a more descriptive error message when the TNotifier constraint is not met, improving the clarity of compilation errors for developers using this operator.

src/rpp/rpp/operators/scan.hpp (1)

153-157: LGTM!

The changes to the scan function improve clarity and type safety without altering its behavior:

  • Renaming the initial_value parameter to seed better reflects its purpose.
  • Updating the template type from InitialValue to Seed maintains consistency with the new naming convention.
  • The updated static assertion ensures that the accumulator function is invocable with the new Seed type and the expected value type, enhancing type safety.

Overall, these modifications enhance the readability and maintainability of the code.

src/rpp/rpp/operators/fwd.hpp (9)

30-31: LGTM!

The introduction of constraint::template_callable_or_invocable enhances the clarity and maintainability of the code by consolidating the checks for callable types and invocability. The change does not introduce any logical errors or alter the behavior of the function.


44-45: LGTM!

The removal of the redundant check involving utils::is_not_template_callable simplifies the constraint and improves the readability of the code. The change does not introduce any logical errors or alter the behavior of the function.


52-53: LGTM!

The removal of the redundant check involving utils::is_not_template_callable simplifies the constraint and improves the readability of the code. The change does not introduce any logical errors or alter the behavior of the function.


58-59: LGTM!

The removal of the redundant check involving utils::is_not_template_callable simplifies the constraint and improves the readability of the code. The change does not introduce any logical errors or alter the behavior of the function.


60-62: LGTM!

The removal of the redundant checks involving utils::is_not_template_callable simplifies the constraints and improves the readability of the code. The change does not introduce any logical errors or alter the behavior of the function.


Line range hint 84-85: LGTM!

The removal of the redundant checks involving utils::is_not_template_callable simplifies the constraints and improves the readability of the code. The change does not introduce any logical errors or alter the behavior of the function.


100-101: LGTM!

The change in parameter type from InitialValue to Seed streamlines the function's interface and improves the clarity of the code. The change does not introduce any logical errors or alter the behavior of the function.


179-180: LGTM!

The introduction of constraint::template_callable_or_invocable enhances the clarity and maintainability of the code by consolidating the checks for callable types and invocability. The change does not introduce any logical errors or alter the behavior of the function.


192-193: LGTM!

The introduction of constraint::template_callable_or_invocable enhances the clarity and maintainability of the code by consolidating the checks for callable types and invocability. The change does not introduce any logical errors or alter the behavior of the function.

src/rpp/rpp/operators/group_by.hpp (1)

220-222: LGTM!

The static_assert statements are well-placed and improve the type safety of the group_by function. The error messages are clear and informative, which helps in identifying and fixing any constraint violations quickly.

src/rpp/rpp/operators/merge.hpp (1)

258-258: Great addition of the static_assert to enforce type consistency at compile-time!

The static_assert using the constraint::observables_of_same_type constraint is a valuable addition as it ensures that all observables passed to the merge_with function are of the same type. This compile-time check will help catch any potential type mismatches early in the development process, improving the type safety and maintainability of the code.

Note that this is a breaking change for any code that was previously passing observables of different types to merge_with, but that would have been a bug anyway. The long-term benefits of the improved type safety outweigh the short-term inconvenience of fixing any such code.

Overall, this is a great change that enhances the robustness of the merge_with function. Well done!

src/rpp/rpp/operators/with_latest_from.hpp (2)

201-201: Constraint refactoring improves code clarity.

The change from the previous condition to the new constraint::template_callable_or_invocable constraint simplifies the code and improves readability without altering the behavior of the function.

This refactoring enhances the maintainability of the codebase by consolidating the logic into a single, more expressive constraint.


Line range hint 230-233: No changes to review.

This overload of with_latest_from has no modifications in the provided code diff. Skipping review.

src/rpp/rpp/operators/subscribe.hpp (1)

212-212: LGTM!

The change to the on_next_like concept simplifies the logic by consolidating the checks into a single concept using rpp::constraint::template_callable_or_invocable<OnNext, rpp::utils::convertible_to_any>. This refinement potentially allows for broader compatibility with callable types while maintaining the necessary constraints.

The exclusion of specific types remains intact, preserving the overall intent of the concept. This change improves the readability and maintainability of the concept.

@AlexInLog AlexInLog closed this Sep 19, 2024
@AlexInLog AlexInLog deleted the static_asserts branch September 19, 2024 20:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant